-
-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Code clarity update #492
Code clarity update #492
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Sent you a couple thoughts on discord |
let me know your thoughts on this since "not exposing" the structs outside the package scope was the point of keeping them camelCase. The idea is that those structs etc are part of the internal API and should not be exposed as they would unnecessarily need to be included in the backward compatibility guarantee slowing down development and also bloating the api documentation. |
12fb92c
to
526c3d4
Compare
|
Improving code clarity I'm starting to make some changes to the code documentation here. I'm including a bit of refactoring to make sure that things are more readable. - The code extensively uses the short variable names to the detriment of the readability. This compounds the problem of not having enough code documentation. - While golang recommends short variable names, the intention is to avoid long nested function calls and to avoid diverting the attention from the main logic of the program. However, even the main logic variables have really short names making it almost impossible to read. Incremental renaming of variables in intro.go First pass of the clarification renaming: 1. Changed all type structs were Pascal Case 2. Expanded 2-3 character field names for structs 3. Tried removing redundant functions Add comments and update function names Add new comments and function names for better code clarity Updated go go project files Undid struct naming that pushed stuff to be public Undid the public struct scoping
526c3d4
to
5157f78
Compare
@dosco I know this took a long time but I basically undid all the public structs and helped rename a lot of the variables to get a better idea about the flow. If this is good, I'll restart the multischema support, we definitely need it on our end |
Thats a huge PR did you manually add these comments or is there a tool that helps? |
The code currently is hard to work with because of the lack of documentation, the crazy number of features pack in and usage of shorthand variables.
This PR tries to improve the situation for the following scenarios: